-
Notifications
You must be signed in to change notification settings - Fork 273
Feat: Implement APIs for Tracking Departed Users with Assigned Tasks #2251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: Implement APIs for Tracking Departed Users with Assigned Tasks #2251
Conversation
- users/departed-users retrieves users with assigned tasks who have left the Discord server Added test cases for the API and fixed test case issues arising from mock data additions used in current API testing.
Are pr 2251 and 2252 stack PR? and In what order I need to review them? |
… and the corresponding queries to model. - removed test cases from integration testing and moved it to unit testing/ - moved archieveUsers to users model as it was causing circular dependency. - created a separate fixture file for mock data required to test the feature. - reverted the test case changes done for other test cases due to mock data changes.
There were 2 api's in the original PR, I have separated both of them into separate PR's. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few comments @VinuB-Dev
- Added function documentation for all the modified and new model functions. - Implemented a negative unit test case for the `fetchUsersNotInDiscordServer` function. - Updated `fetchUsersNotInDiscordServer` to use `userId` as a parameter instead of the entire `user` object. - Added test cases for recent model changes.
Not under feature flag? @VinuB-Dev i |
@vinit717 Should we put these APIs under a feature flag? The APIs are read-only, fetching data from the database without performing any write operations. They are completely independent, involve straightforward database queries, and are not dependent on external factors. Considering their simplicity, I feel we don’t need to place them under a feature flag. |
@VinuB-Dev You have moved archivedUser function to different file. Have you fixed all the imports statement? |
Yes I have and all test cases are working fine. |
…the /users/departed-users API. - Implemented negative test cases for the /users/departed-users endpoint: - Tested the scenario where the dev flag is not provided, expecting a 404 status code with a "Route not found" message. - Tested the scenario where the database query fails, ensuring the API responds with a 500 status code and an appropriate error message.
…v/website-backend into feat/departed-users-api
as this PR is outdated compared to develop, Could you please pull the latest changes from develop into your branch and resolve any conflicts if they arise? |
Done |
…nality under /users route with departed parameter. Added test cases for the same.
controllers/users.js
Outdated
}); | ||
} catch (error) { | ||
logger.error("Error when fetching users who abandoned tasks:", error); | ||
return res.boom.badImplementation(INTERNAL_SERVER_ERROR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will be the status code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll be 500
You can see ti at the end here - https://www.npmjs.com/package/boom/v/2.5.0?activeTab=readme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few comments
controllers/users.js
Outdated
try { | ||
result = await dataAccess.retrieveUsers({ query: req.query }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will be the query here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll be { departed: "true" }
return res.boom.notFound("Route not found"); | ||
} | ||
try { | ||
const result = await dataAccess.retrieveUsers({ query: req.query }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is it returning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An object
{ users: [], nextId: '' , prevId: ''}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please write API contract for this API
@Achintya-Chatterjee Where should I add it? In the description or somewhere else? |
|
Splitting this PR into smaller parts for easier review. This PR will serve as the main split: #2268. Closing this PR as part of the process. |
Date: 15/11/2024
Developer Name: Vignesh B S
Issue Ticket Number
Closes Real-Dev-Squad/website-dashboard#888
Description
Implemented changes in users API for Tracking Departed Users.
users?departed=true
- Retrieves users who had a task assigned and have departed the Discord server.Added test cases for the above api.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes
PRD
Design Doc
Have moved the archieveUsers function to models as it was directly interacts with the database and performing operations such as updating task documents. This was also causing circular dependency issues.